Support SCRIPT_NAME (subdirectory or proxied environment) #104
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #104 +/- ##
==========================================
+ Coverage 97.54% 97.55% +0.01%
==========================================
Files 4 4
Lines 244 245 +1
Branches 67 67
==========================================
+ Hits 238 239 +1
Misses 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for SCRIPT_NAME to properly generate callback URLs when the application is mounted under a subdirectory (e.g., behind a reverse proxy or using Rack::URLMap). The key change is replacing callback_path with callback_url in the LDAP strategy to ensure absolute URLs include the subdirectory prefix.
- Changed
callback_pathtocallback_urlin redirects and form actions - Added comprehensive test coverage for subdirectory mounting scenarios
- Added documentation explaining SCRIPT_NAME support and configuration examples
Reviewed Changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/omniauth/strategies/ldap.rb | Updated to use callback_url instead of callback_path for redirects and form action; refactored filter method to avoid shadowing variable name |
| spec/omniauth/strategies/ldap_spec.rb | Updated existing test expectations and added new test contexts for subdirectory and nested subdirectory scenarios |
| spec/integration/middleware_spec.rb | Added integration test verifying SCRIPT_NAME is honored in redirect to callback |
| README.md | Added comprehensive documentation section explaining SCRIPT_NAME support with Rack and Rails examples |
| CHANGELOG.md | Documented the new feature addition |
| docs/* | Regenerated documentation files with updated timestamps and new content |
| .rubocop_gradual.lock | Updated line number references due to added tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <li id="object_omniauth-ldap-2.3.1.gem" class="even"> | ||
| <div class="item"><span class="object_link"><a href="file.omniauth-ldap-2.3.1.gem.html" title="omniauth-ldap-2.3.1.gem">omniauth-ldap-2.3.1.gem</a></span></div> | ||
| </li> | ||
|
|
||
|
|
||
| <li id="object_omniauth-ldap-2.3.1.gem" class="odd"> | ||
| <div class="item"><span class="object_link"><a href="file.omniauth-ldap-2.3.1.gem.html" title="omniauth-ldap-2.3.1.gem">omniauth-ldap-2.3.1.gem</a></span></div> | ||
| </li> |
There was a problem hiding this comment.
Duplicate list items with the same ID object_omniauth-ldap-2.3.1.gem. HTML IDs must be unique within a document. One of these entries should be removed.
| <li class="r2"><a href="file.omniauth-ldap-2.3.1.gem.html" title="omniauth-ldap-2.3.1.gem">omniauth-ldap-2.3.1.gem</a></li> | ||
|
|
||
|
|
||
| <li class="r1"><a href="file.omniauth-ldap-2.3.1.gem.html" title="omniauth-ldap-2.3.1.gem">omniauth-ldap-2.3.1.gem</a></li> |
There was a problem hiding this comment.
Duplicate list items linking to the same gem file. One of these entries should be removed to avoid redundancy.
| <li class="r1"><a href="file.omniauth-ldap-2.3.1.gem.html" title="omniauth-ldap-2.3.1.gem">omniauth-ldap-2.3.1.gem</a></li> |
- test against all minor versions of omniauth
- test against all minor versions of rack
Minimum allowed line rate is |
Support for SCRIPT_NAME for proper URL generation
Expands on, and replaces, #16